fix(ralph): harden repo-root resolution in verify-hooks-e2e#102
fix(ralph): harden repo-root resolution in verify-hooks-e2e#102
Conversation
📝 WalkthroughWalkthroughThe bash script's repository root resolution logic was refactored to use Git's Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.devagent/plugins/ralph/tools/verify-hooks-e2e.sh:
- Around line 13-16: The git invocation that computes REPO_ROOT should use
SCRIPT_DIR directly instead of the hard-coded "../../../../.." traversal; change
the command that sets REPO_ROOT (the line using git -C and rev-parse) to run git
-C "${SCRIPT_DIR}" rev-parse --show-toplevel so it reliably resolves the repo
root from the script location, keeping the existing error check and
processLogger-style echo/exit behavior intact.
| if ! REPO_ROOT="$(git -C "${SCRIPT_DIR}/../../../../.." rev-parse --show-toplevel 2>/dev/null)"; then | ||
| echo "Error: Unable to resolve repo root from ${SCRIPT_DIR}" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Redundant path traversal undermines the git-based approach.
git rev-parse --show-toplevel resolves the repository root from any location within the repo. The manual ../../../../.. navigation is unnecessary and reintroduces the brittleness you're trying to eliminate—if the script ever moves, this path breaks.
♻️ Proposed fix: use SCRIPT_DIR directly
-if ! REPO_ROOT="$(git -C "${SCRIPT_DIR}/../../../../.." rev-parse --show-toplevel 2>/dev/null)"; then
+if ! REPO_ROOT="$(git -C "${SCRIPT_DIR}" rev-parse --show-toplevel 2>/dev/null)"; then
echo "Error: Unable to resolve repo root from ${SCRIPT_DIR}" >&2
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ! REPO_ROOT="$(git -C "${SCRIPT_DIR}/../../../../.." rev-parse --show-toplevel 2>/dev/null)"; then | |
| echo "Error: Unable to resolve repo root from ${SCRIPT_DIR}" >&2 | |
| exit 1 | |
| fi | |
| if ! REPO_ROOT="$(git -C "${SCRIPT_DIR}" rev-parse --show-toplevel 2>/dev/null)"; then | |
| echo "Error: Unable to resolve repo root from ${SCRIPT_DIR}" >&2 | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In @.devagent/plugins/ralph/tools/verify-hooks-e2e.sh around lines 13 - 16, The
git invocation that computes REPO_ROOT should use SCRIPT_DIR directly instead of
the hard-coded "../../../../.." traversal; change the command that sets
REPO_ROOT (the line using git -C and rev-parse) to run git -C "${SCRIPT_DIR}"
rev-parse --show-toplevel so it reliably resolves the repo root from the script
location, keeping the existing error check and processLogger-style echo/exit
behavior intact.
1 participant
Summary\n- resolve repo root using /Users/jaruesink/projects/devagent anchored from script location\n- add explicit error when repo root cannot be resolved\n- update usage comments to reflect invocation no longer depends on caller cwd\n\n## Why\nThis makes the E2E verifier resilient to caller working directory assumptions and prepares safer extraction from PR #100 without merging it.\n\n## File\n- .devagent/plugins/ralph/tools/verify-hooks-e2e.sh
Summary by CodeRabbit